Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Organization Users page needs responsiveness #10413

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from

Conversation

Tanuj1718
Copy link
Contributor

@Tanuj1718 Tanuj1718 commented Feb 5, 2025

Proposed Changes

Before Update:
Screenshot 2025-02-05 at 12 42 02 PM

After Update:
Screenshot 2025-02-05 at 1 37 22 PM

EDIT 1: Color of Link User button changed. Now it is same as earlier.
EDIT 2: Fixes #10429

@ohcnetwork/care-fe-code-reviewers

Merge Checklist

  • Add specs that demonstrate bug / test a new feature.
  • Update product documentation.
  • Ensure that UI text is kept in I18n files.
  • Prep screenshot or demo video for changelog entry, and attach it to issue.
  • Request for Peer Reviews
  • Completion of QA

Summary by CodeRabbit

  • Style
    • Enhanced layout responsiveness across user management pages, allowing content to wrap gracefully on smaller screens.
    • Adjusted spacing and alignment in add/link user interfaces for a more consistent and cohesive appearance.
    • Optimized display of user details with improved text truncation, ensuring long names are neatly presented.
    • Updated button styling for linking actions, providing a refreshed visual experience.

@Tanuj1718 Tanuj1718 requested a review from a team as a code owner February 5, 2025 11:19
Copy link
Contributor

coderabbitai bot commented Feb 5, 2025

Walkthrough

The changes update the layout and styling across multiple components to improve responsiveness. Modifications include adding the flex-wrap property, adjusting margin classes, and applying text truncation for long usernames. These updates affect the visual presentation without altering any functional logic or public interfaces.

Changes

File(s) Change Summary
src/pages/Organization/OrganizationUsers.tsx, src/pages/Facility/…/FacilityOrganizationUsers.tsx, src/pages/Facility/…/components/LinkFacilityUserSheet.tsx, src/pages/Facility/…/components/EditFacilityUserRoleSheet.tsx Updated layout classes by adding flex-wrap and adjusting flex/margin properties to enhance responsiveness and alignment.
src/pages/Organization/components/AddUserSheet.tsx, src/pages/Organization/components/LinkUserSheet.tsx Modified button styling by removing the ml-3 margin class and updating the button variant to primary_gradient.
src/components/Common/UserSelector.tsx Added the truncate class to manage text overflow in the displayed username.

Assessment against linked issues

Objective Addressed Explanation
Proper responsiveness on Organization Users page (#10405)
Responsiveness on Edit User Page for Organization Users (#10429)

Possibly related PRs

Suggested labels

needs testing, needs review

Suggested reviewers

  • Jacobjeevan
  • rithviknishad

Poem

I'm a little rabbit with a cheerful code hop,
Flex-wraps and margins make UI non-stop.
Truncated names dance on tiny screens,
In responsive fields where perfection gleams.
With every change, I nibble with delight!
Happy coding, and may your layouts shine bright!


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

netlify bot commented Feb 5, 2025

Deploy Preview for care-ohc ready!

Name Link
🔨 Latest commit 837a6c9
🔍 Latest deploy log https://app.netlify.com/sites/care-ohc/deploys/67a7312bb6673400085927a7
😎 Deploy Preview https://deploy-preview-10413--care-ohc.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (4)
src/pages/Facility/settings/locations/LocationView.tsx (4)

98-98: Remove unnecessary flex-wrap property.

The flex-wrap property has no effect on a flex-col container since columns don't wrap. Consider simplifying to:

-      <div className="my-4 flex flex-col flex-wrap">
+      <div className="my-4 flex flex-col">

99-101: Simplify nested flex containers.

While the flex-wrap properties improve responsiveness, the nested structure with multiple flex containers is redundant. Consider simplifying:

-        <div className="py-2 flex flex-wrap justify-between">
-          <div className="flex flex-col flex-wrap justify-between items-start gap-4">
-            <div className="flex flex-wrap items-center gap-4 py-2">
+        <div className="py-2 flex flex-wrap items-start justify-between gap-4">
+          <div className="flex flex-wrap items-center gap-4">

120-120: Remove unnecessary flex properties from search input.

The flex-wrap properties on both the container and input element don't contribute to the layout:

-            <div className="flex flex-wrap w-72">
+            <div className="w-72">
               <Input
                 placeholder={t("search_by_name")}
                 value={searchQuery}
                 onChange={(e) => {
                   setSearchQuery(e.target.value);
                   setPage(1);
                 }}
-                className="w-full flex flex-wrap"
+                className="w-full"
               />

Also applies to: 128-128


139-139: Remove trailing space in className.

The button styling is good for touch targets, but there's an unnecessary trailing space in the className:

-                <Button variant="outline" className="py-2 my-2 ">
+                <Button variant="outline" className="py-2 my-2">
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5d9273d and ea48433.

📒 Files selected for processing (3)
  • src/pages/Facility/settings/locations/LocationView.tsx (3 hunks)
  • src/pages/Organization/OrganizationUsers.tsx (2 hunks)
  • src/pages/Organization/components/AddUserSheet.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (2)
  • src/pages/Organization/components/AddUserSheet.tsx
  • src/pages/Organization/OrganizationUsers.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: cypress-run (1)
🔇 Additional comments (1)
src/pages/Facility/settings/locations/LocationView.tsx (1)

98-151: LGTM! The responsive layout improvements are well implemented.

The changes successfully improve the mobile responsiveness of the location view. While there are some opportunities to optimize the flex properties as noted above, the overall direction is good.

@Jacobjeevan
Copy link
Contributor

Include #10429 as well 👍

@Tanuj1718 Tanuj1718 mentioned this pull request Feb 6, 2025
6 tasks
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes not relevant to the PR; remove.

<div>
<span className="text-sm text-gray-500">{t("username")}</span>
<p className="text-sm font-medium">{userRole.user.username}</p>
<div className="flex flex-col flex-wrap gap-2 pt-2 border-t">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Let's add truncate and tooltip for usernames (refer to other examples in codebase).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Check multiple devices.

@Jacobjeevan
Copy link
Contributor

Include #10429 as well 👍

Add this issue number in the description as well.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/pages/Facility/settings/organizations/components/LinkFacilityUserSheet.tsx (1)

146-152: Consider improving text truncation for user details.

While the container has the truncate class, the name and email might still overflow. Consider adding individual truncation.

 <div className="flex flex-col truncate">
-  <span className="font-medium text-lg">
+  <span className="font-medium text-lg truncate">
     {selectedUser.first_name} {selectedUser.last_name}
   </span>
-  <span className="text-sm text-gray-500">
+  <span className="text-sm text-gray-500 truncate">
     {selectedUser.email}
   </span>
 </div>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2b7ad7d and 837a6c9.

📒 Files selected for processing (4)
  • src/components/Common/UserSelector.tsx (1 hunks)
  • src/pages/Facility/settings/organizations/FacilityOrganizationUsers.tsx (1 hunks)
  • src/pages/Facility/settings/organizations/components/EditFacilityUserRoleSheet.tsx (1 hunks)
  • src/pages/Facility/settings/organizations/components/LinkFacilityUserSheet.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/pages/Facility/settings/organizations/components/EditFacilityUserRoleSheet.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: cypress-run (1)
🔇 Additional comments (3)
src/components/Common/UserSelector.tsx (1)

66-66: LGTM! Good improvement for handling long usernames.

The addition of the truncate class helps prevent text overflow and improves the responsive design by properly handling long usernames.

src/pages/Facility/settings/organizations/FacilityOrganizationUsers.tsx (1)

69-69: LGTM! Good responsive design improvement.

The addition of flex-wrap ensures proper layout on smaller screens by allowing the title and action buttons to wrap when needed, preventing horizontal overflow.

src/pages/Facility/settings/organizations/components/LinkFacilityUserSheet.tsx (1)

139-139: LGTM! Good responsive design improvements.

The changes improve the handling of long text content and ensure proper wrapping on smaller screens:

  • Added flex-wrap for better layout flexibility
  • Added truncate classes to prevent text overflow

Also applies to: 145-145, 156-156

Copy link
Contributor

@Jacobjeevan Jacobjeevan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In OrganizationUsers:

image

Adjust for iPad; make sure to check other devices as well, for any changes you make.

Ensure more details button remains at the bottom

image

Seems to be fine for FacilityOrganizationUsers, so check against that.

Copy link
Contributor

@Jacobjeevan Jacobjeevan Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Let's add tooltip and truncate for this as well (TooltipComponent).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants